Skip to content
This repository has been archived by the owner on May 21, 2024. It is now read-only.

Test/ostree full fake install #1195

Merged
merged 9 commits into from
Apr 29, 2019
Merged

Test/ostree full fake install #1195

merged 9 commits into from
Apr 29, 2019

Conversation

lbonn
Copy link
Contributor

@lbonn lbonn commented Apr 26, 2019

Finally :).

Well, almost:

  • still has funny issues with valgrind
  • would like to try to merge fake_http_server.py and fake_test_server.py, they share most of their code and both have similar non-descriptive names

@lbonn lbonn force-pushed the test/ostree-full-fake-install branch 4 times, most recently from b8c28ac to fecbab4 Compare April 26, 2019 13:56
@lbonn
Copy link
Contributor Author

lbonn commented Apr 26, 2019

Oh it actually looks like valgrind doesn't have any problem on CI... I have something on my debian testing, might come back on ptest, as it usually does.

@lbonn lbonn force-pushed the test/ostree-full-fake-install branch from fecbab4 to bca0838 Compare April 26, 2019 15:08
@codecov-io
Copy link

codecov-io commented Apr 26, 2019

Codecov Report

Merging #1195 into master will increase coverage by 0.17%.
The diff coverage is 86.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1195      +/-   ##
==========================================
+ Coverage   77.36%   77.53%   +0.17%     
==========================================
  Files         168      168              
  Lines        9861     9870       +9     
==========================================
+ Hits         7629     7653      +24     
+ Misses       2232     2217      -15
Impacted Files Coverage Δ
src/libaktualizr/primary/aktualizr.h 100% <ø> (ø) ⬆️
src/libaktualizr/config/config.h 100% <ø> (ø) ⬆️
src/libaktualizr/primary/sotauptaneclient.h 100% <ø> (ø) ⬆️
src/aktualizr_repo/image_repo.h 100% <ø> (ø) ⬆️
src/aktualizr_repo/uptane_repo.cc 96.55% <100%> (ø) ⬆️
src/aktualizr_repo/main.cc 76.92% <71.42%> (-0.29%) ⬇️
src/aktualizr_repo/image_repo.cc 93.63% <90.9%> (+0.11%) ⬆️
src/libaktualizr/package_manager/ostreemanager.cc 77.11% <0%> (+7.2%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c50feb3...cc9f670. Read the comment docs.

@lbonn lbonn force-pushed the test/ostree-full-fake-install branch from bca0838 to ef03877 Compare April 29, 2019 08:07
Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cool! I think I even understand most of what is going on.

TemporaryDirectory treehub_dir;
boost::process::child ostree_server_process("tests/sota_tools/treehub_server.py", std::string("-p"), treehub_port,
std::string("-d"), treehub_dir.PathString(), std::string("-s0.5"),
std::string("--create"));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you still need the treehub_server if you are using the fake_test_server? I'd hoped when I wrote that for it to be able to handle all server tasks in one script.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tried, to be honest. This test is mostly a mix-and-match of other tests that we have.

However:

  • treehub_server also generates a repository at startup which the test uses
  • there is the cgi / multipart business that I don't quite understand.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. It'd be nice to clean up our various test servers, but it doesn't need to be done now. We can come to back to that later if you'd like.

@pattivacek
Copy link
Collaborator

Actually, do you think we could convince OSTree to incorporate the ability to fake a deployment somehow? It seems like something not completely unreasonable, although if it is much more complicated than what you've already done, it may or may not be worth it.

lbonn and others added 6 commits April 29, 2019 11:27
Also, rename addImage to addBinaryImage

Signed-off-by: Laurent Bonnans <[email protected]>
I was using it for a unit test but decided it wasn't actually useful for
that, but it still may be useful for other purposes.

Signed-off-by: Patrick Vacek <[email protected]>
To be added to LD_PRELOAD for advanced mocking

Signed-off-by: Laurent Bonnans <[email protected]>
@lbonn lbonn force-pushed the test/ostree-full-fake-install branch 2 times, most recently from 8af7118 to b24bc4b Compare April 29, 2019 10:12
lbonn added 2 commits April 29, 2019 15:58
With some parts mocked (emulate booted deployments)

Signed-off-by: Laurent Bonnans <[email protected]>
@lbonn lbonn force-pushed the test/ostree-full-fake-install branch from b24bc4b to cb99aa6 Compare April 29, 2019 13:58
@lbonn
Copy link
Contributor Author

lbonn commented Apr 29, 2019

Actually, do you think we could convince OSTree to incorporate the ability to fake a deployment somehow? It seems like something not completely unreasonable, although if it is much more complicated than what you've already done, it may or may not be worth it.

Hm, I don't really see ourselves convincing them to do that work for us, as I don't know if others have similar needs and that's the kind of work that could make the code base more complex without added functionality.

But could be possible if we have a somehow lightweight concrete plan (maybe make some hardcoded paths configurable?) and present a PR in an almost working state. Would require some more investigation on our side, though.

Copy link
Collaborator

@pattivacek pattivacek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But could be possible if we have a somehow lightweight concrete plan (maybe make some hardcoded paths configurable?) and present a PR in an almost working state. Would require some more investigation on our side, though.

I agree wholeheartedly with your statements. The biggest part seems to be configuring hardcoded paths as you mention. Would you mind making a ticket and sharing it (or tagging me) to describe the work of that investigation? We don't need to worry about it now, but it be might worth looking into at a later time and/or if someone gets bored.

And by the way, thanks for merging the two fake servers! And for figuring all of this out in general! Seems quite cool and should enable a lot more extensive tests, too.

One thing I forgot: marking the test as done in actions.md and linking it with a comment. I'm not going to hold this up just for that, though.

@lbonn
Copy link
Contributor Author

lbonn commented Apr 29, 2019

Added updates to actions.md.
Will merge if ok, as CI passed on the previous commit.

@lbonn lbonn merged commit dd95294 into master Apr 29, 2019
@lbonn lbonn deleted the test/ostree-full-fake-install branch April 29, 2019 15:34
@mike-sul
Copy link
Collaborator

Nice work, I think, it's quite useful to fake the ostree work.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants